Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

libgetopts: tweak HasHarg/Occur docs #16252

Merged
merged 1 commit into from Aug 6, 2014
Merged

libgetopts: tweak HasHarg/Occur docs #16252

merged 1 commit into from Aug 6, 2014

Conversation

conradkleinespel
Copy link
Contributor

@conradkleinespel conradkleinespel commented Aug 4, 2014

I found the current docs to be a little confusing. I believe this makes them straight to the point and less confusing.

alexcrichton
alexcrichton reviewed Aug 4, 2014
View changes
src/libgetopts/lib.rs
Optional,
/// The option occurs once or multiple times.
/// The option occurs zero or more times.
Copy link
Member

@alexcrichton alexcrichton Aug 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be one or more?

Copy link
Contributor Author

@conradkleinespel conradkleinespel Aug 4, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it shouldn't. That's partly why I'm making this PR. With Occur set to Multi, it is allowed for the option not to show up at all. With this PR, I'm assuming that the code is correct but the docs a little outdated.

@alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented Aug 4, 2014

Ok, thanks! Could you squash the commits together as well?

@conradkleinespel
Copy link
Contributor Author

@conradkleinespel conradkleinespel commented Aug 4, 2014

Done.

bors
Copy link
Contributor

@bors bors commented on c74d320 Aug 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from alexcrichton
at https://github.com/conradkleinespel/rust/commit/c74d320662329922860c5cdaf8cba1797caebea4

bors
Copy link
Contributor

@bors bors commented on c74d320 Aug 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging conradkleinespel/rust/master = c74d320 into auto

bors
Copy link
Contributor

@bors bors commented on c74d320 Aug 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conradkleinespel/rust/master = c74d320 merged ok, testing candidate = 2a47fa7

bors
Copy link
Contributor

@bors bors commented on c74d320 Aug 6, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 2a47fa7

bors added a commit that referenced this issue Aug 6, 2014
@bors
auto merge of #16252 : conradkleinespel/rust/master, r=alexcrichton
2a47fa7 
I found the current docs to be a little confusing. I believe this makes them straight to the point and less confusing.
@bors bors closed this Aug 6, 2014
@bors bors merged commit c74d320 into rust-lang:master Aug 6, 2014
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@conradkleinespel @alexcrichton @bors